Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Improve the workflow of monitor / decrypting notifications #95

Draft
wants to merge 7 commits into
base: trunk
Choose a base branch
from

Conversation

Xlin123
Copy link
Member

@Xlin123 Xlin123 commented Dec 3, 2023

All of this work is up to discussion, I'm really not sure the best way to handle this stuff.

-What I did
I am mostly brainstorming on how to improve how we actually use monitor / decrypt notifications since right now it's a bit messy.

I created a AtNotificationService that would handle all of the monitor stuff. I left notify in at_client but we can easily move it later.

We now use a generator method in order to yield incoming at_events. In order to thoroughly implement notifications in an app I still suggest to use an additional thread to manage incoming notifications.

I modified handle_event a bit, there was 2 changes:

  1. Added another Queue, decrypted_events, for specifically decrypted notifications.
  2. Added the notify:remove command to get rid of notifications sitting around.

- How I did it

- How to verify it
I was testing it in parallel with sshnpdpy, so here are some examples on how to use the methods.

I'll begin writing unit tests for this.

- Description for the changelog
feat: Improve the workflow of monitor / decrypting notifications

@Xlin123 Xlin123 self-assigned this Dec 3, 2023
@Xlin123 Xlin123 marked this pull request as draft December 3, 2023 01:26
@Xlin123 Xlin123 marked this pull request as ready for review December 3, 2023 02:01
@Xlin123 Xlin123 marked this pull request as draft December 3, 2023 02:02
@Xlin123
Copy link
Member Author

Xlin123 commented Dec 3, 2023

@realvarx I was thinking about abstracting _start_monitor, and _handle_event out to another class. (maybe a util class)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant